I guess this is valid samples? If so, I am actually not sure if we want this or something time-based i.e. allow that not all samples are valid. However, for our current use-case this also works since we would set n=1 anyway.

That's interesting. valid means that any data was received. If a component is missing data, resampler will send None and that is not a valid value.

If a component is sending only None, should this function return after n Nones are received? I'm guessing it should?

I see, in many scenarios I wouldn't distinguish between missing or None values. I think it shouldn't return after n Nones but after n new time steps which have at least 1 real value. However, we could leave this also for later.

I've updated it to return after n samples are received. Whether they were valid or not needs to be checked with a call to count_valid. I've also updated the docs to state this.

Oh, I see the confusion, I understood that it triggers when n new output samples have been "received", i.e. there are timestamps in the resulting moving window. But this is about input samples. So even if we receive 100 samples, if these are all older than newest timestamp we wouldn't get any new timestamp in the window but updated data points of older timestamps.

This makes sense to me, would stress that in the doc though, e.g the valid samples part is confusing IMO since this is indeed about the new samples.

Maybe:

       """Wait until the next `n` samples have been received in the MovingWindow.

        This function returns after `n` input samples have been received, without considering
        whether the received samples are valid or which timestamp they have. The validity of 
        the samples in the updated moving window can be verified by calling the
        [`count_valid`][frequenz.sdk.timeseries.MovingWindow.count_valid] method.

No, it would wait until there are n output samples, but some output samples could be nan.

It does consider the timestamps when the samples are received. It expects n "new" samples to be available in the buffer before it returns.

The current tests only cover cases where there is no resampling in the moving window. I'll rectify that.

Updated to clarify that it returns after n new samples in the moving window, and not just input samples, as discussed.

Oh, I see the confusion, I understood that it triggers when n new output samples have been "received", i.e. there are timestamps in the resulting moving window. But this is about input samples. So even if we receive 100 samples, if these are all older than newest timestamp we wouldn't get any new timestamp in the window but updated data points of older timestamps.

This makes sense to me, would stress that in the doc though, e.g the valid samples part is confusing IMO since this is indeed about the new samples.

Maybe:

       """Wait until the next `n` samples have been received in the MovingWindow.

        This function returns after `n` input samples have been received, without considering
        whether the received samples are valid or which timestamp they have. The validity of 
        the samples in the updated moving window can be verified by calling the
        [`count_valid`][frequenz.sdk.timeseries.MovingWindow.count_valid] method.

This could also just silently wait for self.capacity instead, like slicing when you do [:10] for an array that has less than 10 items. It can be more confusing if you wanted to wait for 10 and got 5 instead but it is something at least familiar in Python. Again maybe @cwasicki can hint which approach would be more intuitive for data scientists.

I thought of it that if n > capacity the n would still be respected, but calculated in terms of time steps since this was triggered. But I think the different understanding here shows already that this could be confusing and we can limit it until we have a case where we need it.

Yeah, true, best to start with a safe approach, for my behaviour you can still easily get is via mw.wait_for_samples(min(n, mw.capacity)) I guess, so it doesn't look like it is adding too much value.

Just thinking out loud, and I don't even think we need to think about it for this PR, but maybe this could be done more efficiently by reversing the logic, and only set the condition when the counter is set (it could be even be a simple Event in this case). This would mean we would need to save the "waiters" in the instance, and then notify the appropriate waiter, so maybe that's a bit costly too, but I guess the most common case will be having only one or very few waiters. 🤔

The most common case is also with n=1 and resampling_interval=15 minutes.

Would say after n new samples are available

done